fix(core): cleanup sensitive info being logged from configuration#1366
Merged
strantalis merged 20 commits intoopentdf:mainfrom Aug 20, 2024
Merged
fix(core): cleanup sensitive info being logged from configuration#1366strantalis merged 20 commits intoopentdf:mainfrom
strantalis merged 20 commits intoopentdf:mainfrom
Conversation
jrschumacher
previously approved these changes
Aug 16, 2024
Member
jrschumacher
left a comment
There was a problem hiding this comment.
This looks like a good start but I think we need a little more to prevent accidental leaks. Areas of concern:
- copying config to an interface and printing
- marshaling and printing
- extra config props that are not mapped to a struct
We can address those at another time.
be7cf0d to
e411ed2
Compare
Member
Author
|
Because we detected performance issues adding masq. I have decided to remove it for the time being. Instead we just use the standard |
Member
|
Specifically, masq used reflection. When logging large objects like request bodies this caused severe performance issues. |
jrschumacher
requested changes
Aug 19, 2024
service/entityresolution/keycloak/keycloak_entity_resolution.go
Outdated
Show resolved
Hide resolved
service/entityresolution/keycloak/keycloak_entity_resolution.go
Outdated
Show resolved
Hide resolved
service/entityresolution/keycloak/keycloak_entity_resolution.go
Outdated
Show resolved
Hide resolved
service/entityresolution/keycloak/keycloak_entity_resolution.go
Outdated
Show resolved
Hide resolved
service/entityresolution/keycloak/keycloak_entity_resolution.go
Outdated
Show resolved
Hide resolved
service/entityresolution/keycloak/keycloak_entity_resolution.go
Outdated
Show resolved
Hide resolved
service/entityresolution/keycloak/keycloak_entity_resolution.go
Outdated
Show resolved
Hide resolved
service/entityresolution/keycloak/keycloak_entity_resolution.go
Outdated
Show resolved
Hide resolved
Co-authored-by: Ryan Schumacher <j.r.schumacher@gmail.com>
Co-authored-by: Ryan Schumacher <j.r.schumacher@gmail.com>
Co-authored-by: Ryan Schumacher <j.r.schumacher@gmail.com>
Co-authored-by: Ryan Schumacher <j.r.schumacher@gmail.com>
Co-authored-by: Ryan Schumacher <j.r.schumacher@gmail.com>
Co-authored-by: Ryan Schumacher <j.r.schumacher@gmail.com>
Co-authored-by: Ryan Schumacher <j.r.schumacher@gmail.com>
293d2e4 to
b376ab8
Compare
jrschumacher
approved these changes
Aug 20, 2024
github-merge-queue bot
pushed a commit
that referenced
this pull request
Aug 20, 2024
🤖 I have created a release *beep* *boop* --- ## [0.4.19](service/v0.4.18...service/v0.4.19) (2024-08-20) ### Features * **core:** add RPCs to namespaces service to handle assignment/removal of KAS grants ([#1344](#1344)) ([ee47d6c](ee47d6c)) * **core:** Adds key ids to kas registry ([#1347](#1347)) ([e6c76ee](e6c76ee)) * **core:** further support in policy for namespace grants ([#1334](#1334)) ([d56231e](d56231e)) * **core:** support grants to namespaces, definitions, and values in GetAttributeByValueFqns ([#1353](#1353)) ([42a3d74](42a3d74)) * **core:** validate kas uri ([#1351](#1351)) ([2b70931](2b70931)) * **policy:** 1277 protos and service methods for Resource Mapping Groups operations ([#1343](#1343)) ([570f402](570f402)) * **sdk:** Load KAS keys from policy service ([#1346](#1346)) ([fe628a0](fe628a0)) * **sdk:** public client and other enhancements to well-known SDK functionality ([#1365](#1365)) ([3be50a4](3be50a4)) ### Bug Fixes * **authz:** Add http routes for authorization to casbin policy ([#1355](#1355)) ([3fbaf59](3fbaf59)) * **core:** align keycloak provisioning in one command ([#1381](#1381)) ([c3611d2](c3611d2)), closes [#1380](#1380) * **core:** align policy kas grant assignments http gateway methods with actions ([#1299](#1299)) ([031c6ca](031c6ca)) * **core:** Autobump service ([#1340](#1340)) ([3414670](3414670)) * **core:** Autobump service ([#1369](#1369)) ([2ac2378](2ac2378)) * **core:** Autobump service ([#1403](#1403)) ([8084e3e](8084e3e)) * **core:** Autobump service ([#1405](#1405)) ([74a7f0c](74a7f0c)) * **core:** bump go version to 1.22 ([#1407](#1407)) ([c696cd1](c696cd1)) * **core:** cleanup sensitive info being logged from configuration ([#1366](#1366)) ([2b6cf62](2b6cf62)) * **core:** policy kas grants list (filter params and namespace grants) ([#1342](#1342)) ([f18ba68](f18ba68)) * **core:** policy migrations timestamps merge order ([#1325](#1325)) ([2bf4290](2bf4290)) * **sdk:** align sdk with platform modes ([#1328](#1328)) ([88ca6f7](88ca6f7)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: opentdf-automation[bot] <149537512+opentdf-automation[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Should Resolve: #751
In this pull request, I add the masq package to provide developers the ability to mask struct fields by applying the tag masq:"secret".
I also removed logging of the services field from the config object. This change forces anyone adding additional services to log their own service-specific configuration separately. The main reason for this is that there are too many unknowns regarding the understanding of the config structure. However, if developers leverage this logger and apply the struct tags, they can redact sensitive fields.
For more information, visit https://github.com/m-mizutani/masq.
Example Loading Config Log
Example Service Log